Skip to content

iOS publish#28

Open
kmsalah wants to merge 11 commits intomasterfrom
iosPublish
Open

iOS publish#28
kmsalah wants to merge 11 commits intomasterfrom
iosPublish

Conversation

@kmsalah
Copy link
Copy Markdown
Member

@kmsalah kmsalah commented May 26, 2020

No description provided.

@kmsalah kmsalah changed the title Ios publish iOS publish May 26, 2020
@kmsalah kmsalah requested review from dmadisetti and pbateman828 May 26, 2020 04:02
Copy link
Copy Markdown
Contributor

@dmadisetti dmadisetti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dope! Super excited for this. Can't wait to see it on the store. Code looks great, seems like it took a lot of tweaking to get just right. Appreciate you for that. I left comments like I would for any bigger project. The only other thing I'd do is sync gameframe with the nom project

node.id = "container"; // Append the text to <li>
document.body.appendChild(node); // Append <li> to <ul> with id="myList"

document.querySelector("#container").innerHTML = window.atob(`{{{html}}}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would set innerHTML prior to appending to page

document.body.style.backgroundColor = "black";

document.querySelector("#container").innerHTML = window.atob(`{{{html}}}`);
var node = document.createElement("DIV"); // Create a <li> node
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean up comments + make DIV -> div



Check out the [CarolinaIgnites coding editor ](https://editor.carolinaignites.org) to create and player your own games.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm haven't tested, but you might have to make these links [text](link)

_refresh(widget.game);
double width = MediaQuery.of(context).size.width;
double adjustment = (width > VIEW_SIZE) ? (width - VIEW_SIZE) / 2 : 0;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd just run a flutter format to clean up some of these

double width = MediaQuery.of(context).size.width;
double adjustment = (width > VIEW_SIZE) ? (width - VIEW_SIZE) / 2 : 0;

if (width > 1000) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm! Even more adjusted? Make me wonder what this page looks like on iPad. Would move this magic number to constants.dart, and maybe use a known break point number:

http://devfacts.com/media-queries-breakpoints-2020/

Looks like 992, 1024 (for iPads) or 1200


login() async {
_signedIn = (await GamesServices.signIn()) == "success";
print(_signedIn);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd clean up some prints, and leave what's remaining as verbose and as debugPrints

if (caller == "QR" && _signedIn) {
analytics.logUnlockAchievement(id: QR_ACHIEVEMENT);
GamesServices.unlock(achievement: Achievement(androidID: QR_ACHIEVEMENT));
GamesServices.unlock(achievement: Achievement(androidID: QR_ACHIEVEMENT, iOSID: QR_ACHIEVEMENT));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this works? If it doesn't break things then cool

@override
Widget build(BuildContext context) {
var devicePhysicalPixelWidth = MediaQuery.of(context).size.width * MediaQuery.of(context).devicePixelRatio;
var headerPadding = (devicePhysicalPixelWidth > 1000) ? MediaQuery.of(context).size.width * 0.15 : 64;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move some of these magic numbers to constants.dart with nice variable names. Linda hard for me to fully image this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants